Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verilog: support virtual interface variables #3720

Merged
merged 2 commits into from
May 13, 2023

Conversation

hirooih
Copy link
Contributor

@hirooih hirooih commented May 12, 2023

virtual interface is a variable. But the Verilog parser mistakenly declared it as an interface.
This PR fixes the issue.

@hirooih hirooih requested a review from masatake May 12, 2023 12:26
@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.02 ⚠️

Comparison is base (6b06565) 82.99% compared to head (ebb1bde) 82.98%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3720      +/-   ##
==========================================
- Coverage   82.99%   82.98%   -0.02%     
==========================================
  Files         226      226              
  Lines       54972    55003      +31     
==========================================
+ Hits        45626    45645      +19     
- Misses       9346     9358      +12     
Impacted Files Coverage Δ
parsers/verilog.c 98.45% <100.00%> (+<0.01%) ⬆️

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -132,6 +133,7 @@ typedef struct sTokenInfo {
bool classScope; /* Context is local to the current sub-context */
bool parameter; /* parameter which can be overridden */
bool hasParamList; /* module definition has a parameter port list */
bool virtual; /* has virtual */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't care about whitespaces in code that I don't maintain directly.
However, you can care:-).

image

If you use Emacs, M-x whitespace-mode helps you see how the sequence of whitespace is made.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masatake san,

However, you can care:-).

Sorry. I have been caring about it.

The indentation rules for this project are different from those I usually use.
I worked on a new PC that had not yet been configured for this project. I wrongly indented the code by hand and committed it.

I have corrected the original commit.

I was also concerned about the inconsistent use of spaces in the indentation. I took this opportunity to add a commit that fixes that as well.

Could you review it again?

Copy link
Member

@masatake masatake May 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About the spacing style(?), I don't have a strong preference.
If the file's maintainer feels comfortable, that's good.

As a reference, we have .editorconfig globally used in our code base.
If you have not introduced editorconfig on your new PC, this will be a good time to introduce it.

I have adjusted the spacing style for only a few files in our source tree because such an adjustment may make 'git bisect' harder.

@masatake masatake added this to the 6.1 milestone May 12, 2023
Signed-off-by: Hiroo HAYASHI <24754036+hirooih@users.noreply.github.com>
Signed-off-by: Hiroo HAYASHI <24754036+hirooih@users.noreply.github.com>
@hirooih hirooih force-pushed the verilog-virtual-interface branch from 55bfbe4 to ebb1bde Compare May 13, 2023 03:38
@hirooih hirooih merged commit dbeed62 into universal-ctags:master May 13, 2023
@hirooih
Copy link
Contributor Author

hirooih commented May 13, 2023

Thank you for your reviewing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants